-
Notifications
You must be signed in to change notification settings - Fork 154
Port Cvar_Prefix.h and ConfigUpdater from SoH #1339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
9995571 to
46f4802
Compare
45fedd6 to
c3a10dc
Compare
Added "CVAR_PREFIX_GAMEPLAY_STATS" and "CVAR_PREFIX_TIME_DISPLAY" to "2ship-cvars.cmake"
Added Include "Cvar_Prefixes.h" to "AudioCollection.cpp" and "AudioEditor.cpp"
Changed code that uses "gOpenWindows." to the "CVAR_WINDOW" macro.
Removed "CVAR_PREFIX_RANDOMIZER_ENHANCEMENT" macro. Removed "CVAR_PREFIX_RANDOMIZER_SETTING" macro. Added "CVAR_RANDOMIZER" macro to "cvar_prefixes.h" Kept "CVAR_RANDOMIZER_ENHANCEMENT" and "CVAR_RANDOMIZER_SETTING" macros for future use. Changed code that uses "gRando." to the "CVAR_RANDOMIZER" macro.
Changed lines that used "CFG_TRACKER_ITEM" to the "CVAR_TRACKER_ITEM" macro.
"CVAR_TRACKER_CHECK" macro in "CheckTracker.cpp".
"gWindows", and "ItemTracker" Added "gTrackers" to "defaultsPresetJ" and "TagMap".
the "CVAR_SETTING("Graphics.AdvancedResolution.*") macro.
…LS("CollisionViewer.*")" macro.
…ntLog.*")" macro.
Cvar_Prefix macro Added a Include guard to "cvar_prefixes.h"
Changed code that uses "CVAR_SETTING("TimeSplits.*)" macro to use the
"CVAR_TRACKER_TIMESPLITS" macro.
"CVAR_SETTING("Notifications.*)" macro
"CVAR_SETTING("DisplayOverlay.*)" macro.
"CVAR_SETTING("Graphics.InterpolationFPS")" macro.
"CVAR_SETTING("Graphics.MatchRefreshRate")" macro.
"gEventLog", and "gNotifications".
"CVAR_AUDIO("*")" macro.
"CVAR_ENHANCEMENT("*")" macro.
"CVAR_DEVELOPER_TOOLS("*")" macro.
"CVAR_TRACKER_ITEM("*")" macro.
"CVAR_WINDOW("*")" macro.
"CVAR_SETTING("*")" macro.
"cvar_prefixes.h" Add #include "2s2h/cvar_prefixes.h" to to files that uses a Cvar_Prefix macro.
64ce3ff to
245c6dc
Compare
|
Hey @Glought thanks for the PR! Sorry for such a long delay on a response and to have left you updating this multiple times. As soon as this went up it sparked some discussion in internal channels but they kind of lingered for a while, and I've landed on wanting to hold off on bringing this pattern over to 2ship. While there is an obvious benefit of enforcing the common cvar prefixes, it's not as large of a problem on 2ship as it ever was on SoH because of our more limited and careful placement of cvars, and the problem is slowly getting smaller on SoH even as they adapt some of the newer patterns. IMO it makes cvar usage a bit more clunky and my editor is constantly fighting it on SoH with these macros not evaluating correctly or whatever. I want to revisit the entirety of CVars and config usage because there have been several pain points we've had to work through over the years, but that's a much bigger undertaking and beyond the scope of this PR. Once again we appreciate the PR along with your others! But we're going to pass on this one for the time being. |
|
Ty for letting me know your decision |
I have port over Cvar_Prefix.h from SoH.
While working on my other PR #1325 I saw inconsistency with the Cvar naming
Example
In BenMenu.cpp:
I thought I'll help by implementing how its done in SoH.
With this lines like
Before:
After
Moved Cvars:
I have reorganized CVars
Configuration Migration
To make this as seamless I have ported over ConfigUpdater from SoH.
I have set it up to Migrate the Cvars I moved.
Here is a "2ship2harkinian.json" file for testing it has everything enabled.
2ship2harkinian.json
Build Artifacts